-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 add dependency bot updates #2957
🌱 add dependency bot updates #2957
Conversation
71c774c
to
f7c9f66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great initiative! I think this will be really helpful in the future and save lots of work.
.github/workflows/dependabot.yml
Outdated
# Workflow files stored in the | ||
# default location of `.github/workflows` | ||
directory: "/" | ||
schedule: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An additional configuration that could be useful could be
commit-message:
prefix: ":seedling:"
As used by cluster-api here: https://github.com/kubernetes-sigs/cluster-api/blob/62e6600244ee7503eefba4db56ef7e94773b8314/.github/dependabot.yml#L7-L8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the changes @oscr mentioned in their review comments
.github/workflows/dependabot.yml
Outdated
# Workflow files stored in the | ||
# default location of `.github/workflows` | ||
directory: "/" | ||
schedule: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See that we've introduced a couple of labelling before merging a PR, what about introducing label ?
One use case maybe ok-to-test
:
- package-ecosystem: "github-actions"
...
labels:
- "ok-to-test"
(In case if it feels necessary to run e2e tests before merging..)
f7c9f66
to
9a6d8f0
Compare
All changes are done succefully. |
schedule: | ||
interval: "weekly" | ||
labels: | ||
- "ok-to-test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work for go dependencies used by the testdata? When I bumped ginkgo, make test
failed until I ran make generate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label only allows the tests be executed. Just that.
Also, note that this action is just to open PR's with updates in the github actions. So, it has no relation with make generate at all. The make generate re-generate the samples projects scaffold under testdata to ensure that the scaffolds matches with the latest changes (it only fails/or is required when you do prs to change the scaffolds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. My thought was if dependabot for example bumps ginkgo (~go get -u github.com/onsi/ginkgo/v2
) and makes a pr, then GH actions will automatically run make test
and it would fail. But this isn't a big worry for me and I could be wrong here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (avoiding lgtm
command to allow other reviewers to approve prior to merging)
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, everettraven The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
HI @everettraven, Thank you for the help. In this case, this PR is very small, it does not promote any risk for the users and etc. Also, we can do follow-ups to improve things always. All changes/suggestions are done then, (ihmo) I think it is good to fly. WDYT? |
/lgtm |
/lgtm |
Description
add dependency bot updates
Motivation
Help us check what is outdated